Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes undefined properties error in tests (closes issue #425) #432

Merged
merged 2 commits into from
Jul 1, 2022

Conversation

klougod
Copy link

@klougod klougod commented Jun 22, 2022

No description provided.

@klougod klougod changed the title Fixes undefined properties eror in tests (closes issue #425) Fixes undefined properties error in tests (closes issue #425) Jun 29, 2022
@klougod
Copy link
Author

klougod commented Jun 29, 2022

@dilanx Hello,

Here is the PR as requested in issue #425, hope it is fine to merge, if there's any update or changes just comment and I'll correct it immediately.

Thanks

@dilanx dilanx removed their request for review June 29, 2022 14:54
@dilanx
Copy link
Owner

dilanx commented Jun 29, 2022

Sorry about the delay, @klougod.

Is this update specifically for versions prior to craco 7? Are you able to recreate your original issue on the alpha version of craco?

Because CRA 5 is over a year old now already, it may be best to stop supporting CRA 4 (craco 6) in favor of CRA 5 (craco 7). Many packages only support CRA 5 now anyways. I wouldn't want to merge your PR if it'll just be overwritten soon by the new update.

If the intent is that this is just for craco 6 (which will soon stop being supported), I can go ahead and merge and release a new craco 6 version.

@klougod
Copy link
Author

klougod commented Jun 29, 2022

Sorry about the delay, @klougod.

Is this update specifically for versions prior to craco 7? Are you able to recreate your original issue on the alpha version of craco?

Because CRA 4 (craco 6) is over a year old now, it may be best to stop supporting it in favor of CRA 5 (craco 7). Many packages only support CRA 5 now anyways. I wouldn't want to merge your PR if it'll just be overwritten soon by the new update.

If the intent is that this is just for craco 6 (which will soon stop being supported), I can go ahead and merge and release a new craco 6 version.

I'm having this problem with craco 6 but i'll surely try to reproduce it with craco 7, soon i'll give an update here

@klougod
Copy link
Author

klougod commented Jul 1, 2022

@dilanx Hi,

As I expected this only happens in craco version 6, the intent for this PR would be to fix it only for the version 6, in my case and many others it would help since I cant fully update to craco 7.

Thanks

@dilanx
Copy link
Owner

dilanx commented Jul 1, 2022

Okay sounds good, I'll merge your changes and release a patch for version 6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Craco with jest 27 createTransformer and _cracoConfig undefined problem after updating from v4
3 participants